Move the join-based narrowing logic towards a union-based narrowing logic. #18212
Conversation
…ogic. More concretely: use `make_simplified_union` instead of `join_simple` in `ConditionalTypeBinder.update_from_options`
for more information, see https://pre-commit.ci
|
Just a draft, I did not find the button to mark it accordingly... |
This comment has been minimized.
This comment has been minimized.
…ing_towards_union_based_narrowing' into narrowing/move_join_based_narrowing_towards_union_based_narrowing
This comment has been minimized.
This comment has been minimized.
tyralla
added
topic-type-narrowing
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: pydantic (https://github.com/pydantic/pydantic)
+ pydantic/v1/main.py:901: error: Set comprehension has incompatible type Set[int | str]; expected Set[str | None] [misc]
+ pydantic/v1/main.py:901: note: Left operand is of type "set[str] | dict_keys[str, Any]"
pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/computation/expr.py:680: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/computation/expr.py:680: error: Item "Attribute" of "Attribute | Name" has no attribute "id" [union-attr]
+ pandas/core/computation/expr.py:680: note: Error code "union-attr" not covered by "type: ignore" comment
+ pandas/core/computation/expr.py:702: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/computation/expr.py:702: error: Item "Attribute" of "Attribute | Name" has no attribute "id" [union-attr]
+ pandas/core/computation/expr.py:702: note: Error code "union-attr" not covered by "type: ignore" comment
mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ pymongo/synchronous/cursor.py:1002: error: Invalid index type "int | Any | Pattern[Any]" for "list[Any]"; expected type "SupportsIndex" [index]
+ pymongo/asynchronous/cursor.py:1004: error: Invalid index type "int | Any | Pattern[Any]" for "list[Any]"; expected type "SupportsIndex" [index]
|
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
I don't like this approach, it is way too ad hoc. A more principled solution would be to not use joins at all in update_from_options(). But this will likely cause a bigger fallout, and will also require adjusting some edge cases.
This would be more consistent with ternary expressions, and IIRC this is the last place join_simple() is used. Ideally join/meet should be only used by the constraint solver.
Btw I just checked and there are very few left (apart from the one being discussed), couple ad-hoc uses in |
|
If "move towards..." is not "principled" enough, I better close this PR. I could try a complete step later (and learn a little about the edge cases of narrowing), but please do not hesitate to do it yourself if you have the time and interest (or if you expect less work when implementing than when reviewing it). |
2 participants
Fixes #16379
Only a modest change. PR #18180 brought me to issue #18238. Trying to fix the latter revealed inconsistencies between join-style and union-style narrowing, so I decided to deal with this issue first. And here is the (maybe too simple?) solution...
Regarding the primer output: For
pandas, a (possibly debatable) false negative turns into a similar one. The affectedpydanticandpymungofunctions are complicated and not stringently typed. I would say the new error messages are okay, but I am not 100% sure.